-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
out with autotools, in with cmake #9
base: master
Are you sure you want to change the base?
Conversation
Hey! This looks great. I think a few things are still missing here, though. For example, it would have been nice to have a helper script with some functions that abstract away some of the boilerplate, like the creation of a VMOD shared library. Additionally, tests need a few more properties, so they might also be better off as a function. For example, tests can have timeouts, skip return-codes, working directory and you can even mark a test as will-fail. Varnishtest currently uses the return code 77 for skipped tests. Tests will have the name test01 which doesn't explain much, and it also prevents a bigger build system. Here we could do something like On that note I would also replace CMAKE_SOURCE_DIR with CMAKE_CURRENT_SOURCE_DIR, and I think that should be enough. I'm not sure if changing the binary dir also makes sense, but I guess that's just one of those things you will know at some point if you try to use this inside a bigger build system. It is not currently possible to add VSCs. Make sure that all the nice-to-have compiler flags are present: -fno-omit-frame-pointer so that the backtraces have better chances of working out-of-the-box. I think autotools adds _GNU_SOURCE, but I'm pretty sure that CMake doesn't! Regardless, looks great :) |
Thanks @fwsGonzo , that is awesome feedback, let's have a list to make sure we have everything:
That's quite the list, but feel free to pile on if you think some are missing. And of course, since you are the |
I'll have a look when I find some time. It looks great. |
@fwsGonzo , poke :-) |
Hey, thanks for the reminder! I've been finishing and moving into my new house the last few weeks and it's been very time consuming. I'll be having a go at this next week for sure! |
From my PR at gquintard#1: There are 2 things that may have to be done to make this the ultimate vmod.cmake:
Something like that. |
it's the week-end, let's try something new!
I do realize that
vcdk
is the preferred method now, but having a lean repo just onegit clone
away is still very appealing to me.This is mainly a test run for me to get accustomed to
cmake
, but I really liked what I saw and thought I should share. I opted against a dualautotools
/cmake
to show how much cruft we could remove, but I'm not against it.@dridi, alternatively, that can be used to add a plugin to
vcdk
, feel free to run with it if you prefer that approach